fix(audit): hash created_at at the precision Postgres stores - #2638
Conversation
`log_inner` stamped entries with `Utc::now()` (nanoseconds) and hashed the value via `to_rfc3339()`, whose sub-second digit count follows the value. `audit_log.created_at` is `TIMESTAMPTZ` — microsecond resolution — so `verify_chain` recomputed the digest over a different byte string than the one that was written and returned `HashMismatch` on untampered data. Every chain backed by a real database failed verification at its first entry, which also made a genuinely forged row indistinguishable from the baseline failure. Reduce `created_at` to the stored precision before hashing, so the in-memory entry and the row are byte-identical. `log_inner` is the only place that assigns it — every caller goes through `NewAuditEntry`, which carries no timestamp — so this is a single choke point. Tests: three Postgres-free tests in `hash.rs` pinning the invariant, the trap that caused it, and the round trip; plus a non-ignored `service.rs` test that the write path's timestamp carries no sub-microsecond digits, so a regression is caught by `just test-unit` rather than only by the `#[ignore]` chain tests. Rows written before this stay unverifiable — they always were — so no migration is needed, but an operator relying on an existing chain has to re-anchor. Fixes block#2637 Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
09d3868 to
ebfcb93
Compare
|
P0 triage — merge candidate. 🐝 Reviewed the diff. The root cause is real and the fix is correct:
One thing worth confirming before merge: that every write path routes |
Truncating at the write path fixes the live break, but leaves the invariant as a convention: a future write path that stamps `created_at` without `to_storage_precision` reintroduces the write/read preimage split. Normalize at the single point that consumes the value instead, so the enforcement is structural. Truncation is idempotent and the write path already truncates, so no digest changes — verified by writing a chain with the previous commit and validating those same rows under this one. The `assert_ne!` test that pinned the trap moved down a level: it now asserts on the RFC-3339 preimage (`.123456789` vs `.123456`), which is the property that actually causes the split, and a new test asserts `compute_hash` yields the same digest either way. Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
|
Both points addressed — the second one by taking your suggestion. Every write path. Confirmed: there is exactly one. The enforcement point. You were right that convention isn't enough, so I did it: 08bcce9 moves the normalization into hasher.update(
to_storage_precision(entry.created_at)
.to_rfc3339()
.as_bytes(),
);A future write path that forgets to truncate now can't split the preimage — write-time hashes Digest stability, verified rather than argued. Since the write path already truncates, normalizing again should be a no-op on real chains — but "should be" isn't good enough for a hash function, so I checked it against Postgres: wrote a 3-entry chain with the previous commit, then verified those same rows with this one via a throwaway integration test. So this commit is digest-compatible with the one before it — no re-anchoring needed on top of what the first commit already required. Test restructure.
Re-ran everything after the change: 13 unit tests pass without Postgres, 6/6 chain tests pass against Postgres 17, clippy |
There was a problem hiding this comment.
🤖 Walked through this one carefully and I'm happy with it. Verified the root cause at source: service.rs stamps Utc::now() at nanosecond precision while hash.rs digests created_at.to_rfc3339(), so the write-time preimage carries 9 fractional digits vs 6 after the Postgres µs round-trip — the write/verify split is real.
What I particularly like about this fix:
- It truncates at the write path and normalizes inside
compute_hash, so a future caller that forgets to truncate can't reintroduce the divergence. And sincetrunc_subsecs(6)is idempotent, digests for existing rows are unchanged — I walked the legacy-row verification path and behavior is identical. - The Postgres-free unit test (
log_timestamp_carries_no_sub_microsecond_digits) directly guards the write path, so the fix itself is pinned by a test, not just the bug.
CI is fully green. Thanks for the thorough validation against real Postgres — approving.
## Buzz Relay release v0.2.1 ### Changes since relay-v0.2.0: - fix(sdk): preserve self-mention p tags in message and forum event builders ([#4975](#4975)) ([`78c87ae20e`](78c87ae)) - feat(desktop): adding rich link previews to messages ([#3818](#3818)) ([`1922d49cb2`](1922d49)) - feat(relay): accept kind:30179 private managed-agent events at ingest ([#5133](#5133)) ([`ad923353a2`](ad92335)) - fix(media): require authenticated reads ([#4610](#4610)) ([`769ac70b74`](769ac70)) - feat(identity): recover desktop identity from a signed-in phone ([#4845](#4845)) ([`6eb65919f1`](6eb6591)) - ci: prove the relay-driven mesh lifecycle — discover, join, infer, deny — with real nodes ([#3862](#3862)) ([`38bf642fcf`](38bf642)) - relay: fuzz WebSocket 1012 restart-close timing on graceful drain (BUZZ_DRAIN_JITTER_MS) ([#4542](#4542)) ([`e14fff74d0`](e14fff7)) - fix(reactions): support max-length custom emoji ([#3833](#3833)) ([`2ea9385015`](2ea9385)) - fix(channels): restrict private-channel invitations ([#4612](#4612)) ([`efe1893dd3`](efe1893)) - fix(workflow): bind trigger author to the signed event ([#4607](#4607)) ([`885bed35ee`](885bed3)) - fix(git): revoke access for banned relay members ([#4608](#4608)) ([`997b8caaa4`](997b8ca)) - Define private managed agent wire protocol ([#4593](#4593)) ([`067c085f37`](067c085)) - perf(relay): index channel-id lookups and skip trace-only reads ([#4647](#4647)) ([`bc9e6528a7`](bc9e652)) - Polish mobile inbox and media flows ([#4512](#4512)) ([`feccf4eabc`](feccf4e)) - fix(git): allow deleting the default branch ([#4297](#4297)) ([`fc598f5f8d`](fc598f5)) - feat(projects): add buzz projects CLI commands (NIP-MP kind:30621) ([#4020](#4020)) ([`b7bb15122e`](b7bb151)) - perf(relay): serve relay-membership checks from the read replica ([#4124](#4124)) ([`ac4fa13b8e`](ac4fa13)) - fix(relay): allow open relays to set their NIP-11 workspace icon (kind:9033) ([#3998](#3998)) ([`5765fc74b7`](5765fc7)) - feat(relay): accept kind:30621 multi-repo projects at ingest ([#3171](#3171)) ([`cb9701cd30`](cb9701c)) - feat(relay): raise hosted community limit to five ([#3829](#3829)) ([`10d5a26414`](10d5a26)) - fix(relay): align NIP-11 max_limit with REQ ceiling ([#3635](#3635)) ([`23f0c26b1c`](23f0c26)) - feat(relay): gate kind 30178 team-catalog reads behind the shared tag ([#3358](#3358)) ([`114d40d9d3`](114d40d)) - fix(db): isolate usage metrics advisory-lock test on scratch DB ([#3670](#3670)) ([`dba97eecd9`](dba97ee)) - perf(presence): reduce heartbeat frequency ([#3783](#3783)) ([`bf139e8d0b`](bf139e8)) - feat(mesh): upgrade embedded mesh to v0.74 and harden shared compute (split 1/2 of #3467) ([#3741](#3741)) ([`4933672eb4`](4933672)) - feat(replica): portable heartbeat-token fence with snapshot-local reader routing ([#3268](#3268)) ([`63496cc1d4`](63496cc)) - fix(git): channel binding tooling + author remediation for unbound repos ([#3626](#3626)) ([`788b3c002b`](788b3c0)) - feat: configure S3 URL addressing style ([#3400](#3400)) ([`7012d86d52`](7012d86)) - feat(tracing): correlate trace IDs in relay logs ([#3608](#3608)) ([`005b5b819a`](005b5b8)) - fix(relay): avoid subscription lock inversion ([#3413](#3413)) ([`22be8bb351`](22be8bb)) - feat(cli): add users set-status command for NIP-38 profile status ([#3253](#3253)) ([`60158fce3e`](60158fc)) - feat(relay): make Postgres pool size configurable, default 50 ([#3191](#3191)) ([`2ce2d71cc3`](2ce2d71)) - feat(tracing): add datastore tracing plumbing ([#2760](#2760)) ([`e94b9aeda0`](e94b9ae)) - feat(invites): add use-limited invite links ([#3141](#3141)) ([`d500c2d5cf`](d500c2d)) - feat(admin): show reported message content in report detail ([#3149](#3149)) ([`f069a85503`](f069a85)) - resolve findings ([#3150](#3150)) ([`9b0f744804`](9b0f744)) - Revert "fix(cli,relay): resolve agents by verified owner" ([#3168](#3168)) ([`a041e2d21e`](a041e2d)) - fix(cli,relay): resolve agents by verified owner ([#2615](#2615)) ([`c3084b36d9`](c3084b3)) - fix(security): enforce durable community ban on NIP-43 relay-admin kinds 9030-9033 ([#3128](#3128)) ([`e2e0079101`](e2e0079)) - fix(security): authorize kind:9000 role changes in both directions ([#3017](#3017)) ([`00ecf2cac7`](00ecf2c)) - feat(desktop): handle project work from Inbox ([#3117](#3117)) ([`c5c4f390b6`](c5c4f39)) - feat(relay): make per-owner community limit configurable via BUZZ_MAX_COMMUNITIES_PER_OWNER ([#2599](#2599)) ([`2a051a404d`](2a051a4)) - feat(relay): add author-only-unless-shared read gate for kind 30175 ([#2768](#2768)) ([`ab3af82871`](ab3af82)) - fix(core): block IPv6 transition SSRF targets ([#2801](#2801)) ([`c26bf5945d`](c26bf59)) - fix(workflow): bypass system proxies for webhooks ([#2800](#2800)) ([`60a171b19e`](60a171b)) - fix(audit): hash created_at at the precision Postgres stores ([#2638](#2638)) ([`264a56a226`](264a56a)) - feat(desktop): make pull request reviews actionable ([#2510](#2510)) ([`9081ab0ec9`](9081ab0)) - fix(relay): decompress gzip-encoded git smart-HTTP request bodies ([#2670](#2670)) ([`5ca36e7b91`](5ca36e7)) - fix(sharing): preserve agent/team snapshot tEXt chunks through media sanitization ([#2438](#2438)) ([`b096b0a15a`](b096b0a)) - fix(relay): send 1012 restart close to all clients on graceful drain ([#2575](#2575)) ([`1911c69aa2`](1911c69)) - fix(media): sanitize animated image uploads ([#2524](#2524)) ([`8f8f5fa5a4`](8f8f5fa)) - fix(channels): strip leading hash prefixes from names ([#2250](#2250)) ([`d0ab3fdb05`](d0ab3fd)) - feat(relay): make Redis pool size configurable, default 16 ([#2521](#2521)) ([`bcc3e13069`](bcc3e13)) - feat(desktop+acp): spawn a harness per (agent, community) pair at GUI startup — warm sockets, lazy LLM pool ([#2122](#2122)) ([`61cc738ee8`](61cc738)) - feat(media): add S3-truth per-community storage sweep ([#2044](#2044)) ([`bd37a4d584`](bd37a4d)) - feat(relay): log NIP-98 pubkey attribution on HTTP bridge requests ([#2206](#2206)) ([`7e34bee62c`](7e34bee)) - Revert "feat(relay): inventory unreachable Git objects" ([#2275](#2275)) ([`0fb820f9bf`](0fb820f)) - feat(relay): inventory unreachable Git objects ([#2264](#2264)) ([`3afc9dae15`](3afc9da)) - relay: add author_type label to buzz_events_stored_total ([#2243](#2243)) ([`b9f54c43fe`](b9f54c4)) - fix(git): make project branch workflows reliable ([#2213](#2213)) ([`166f27be4b`](166f27b)) - feat(cli): manage repository protection rules ([#2193](#2193)) ([`f94324598d`](f943245)) - feat(cli): add agents archive/unarchive/archived subcommands ([#2173](#2173)) ([`7d7992067b`](7d79920)) - fix(mobile): sanitize Android image uploads ([#2188](#2188)) ([`ee21da90bd`](ee21da9)) - fix(cli): paginate channel directory queries ([#2181](#2181)) ([`03fe19d603`](03fe19d)) - fix(mobile): image upload fails due to unstripped metadata ([#2185](#2185)) ([`37f15b2001`](37f15b2)) - perf(relay): compact Git packs before manifest limits ([#2172](#2172)) ([`80e0ab16b0`](80e0ab1)) - perf(relay): cache Git pack hydration ([#2169](#2169)) ([`a4d82ec722`](a4d82ec)) - fix(relay): bound and observe Git read operations ([#2167](#2167)) ([`5f7c93d9c1`](5f7c93d)) - relay: gate push enqueue on live leases; batch matcher pipeline (T1b/T1a-repair/T2b) ([#2145](#2145)) ([`e43b2d5aac`](e43b2d5)) - relay: add audit logging disable switch ([#2134](#2134)) ([`bf5acabdde`](bf5acab)) - relay: skip TTL deadline bump for known-permanent channels (T1a write-amp) ([#2125](#2125)) ([`2e936d439c`](2e936d4)) - fix(git): carry NIP-OA delegation in auth event ([#2120](#2120)) ([`c12257d57a`](c12257d)) - Route lag-tolerant reads to an optional Postgres read replica ([#2084](#2084)) ([`29c48883d3`](29c4888)) - fix: recover community access visibility ([#2074](#2074)) ([`ca384d082d`](ca384d0)) - feat: proxy feedback-scoped admin attachments ([#2059](#2059)) ([`d7f918e3cb`](d7f918e)) - feat: add read-only deployment moderation dashboard ([#1999](#1999)) ([`68e670e001`](68e670e)) - Bug-bash round 2: table scroll, Goose instructions, workflow mention wake ([#2034](#2034)) ([`64b8fea6dc`](64b8fea)) - Strip media metadata on clients and reject it at the relay ([#2006](#2006)) ([`5cfd69cb0c`](5cfd69c)) - [codex] Hold Git concurrency permits through streaming (BUZZ-SEC-018) ([#1916](#1916)) ([`7baea42abb`](7baea42)) - [codex] Enforce shared relay admission limits (BUZZ-SEC-019) ([#1917](#1917)) ([`73fc0ec6cf`](73fc0ec)) - [codex] Block banned actors from moderation commands (BUZZ-SEC-007) ([#1915](#1915)) ([`caa195ca58`](caa195c)) - [codex] Fix relay WebSocket admission limits ([#1682](#1682)) ([`d3ce971fc7`](d3ce971)) - feat: add invite QR and mobile direct join ([#1957](#1957)) ([`648cbf3610`](648cbf3)) - fix(join-policy): require legal consent on hosted invites ([#1987](#1987)) ([`2e1577f76f`](2e1577f)) - [codex] Prevent actor-tag UI impersonation ([#1931](#1931)) ([`c540ec9678`](c540ec9)) - Scope relay runtime state by community ([#1658](#1658)) ([`d52dedb06f`](d52dedb)) - Apply optional relay join policy across join flows ([#1894](#1894)) ([`6c2d667575`](6c2d667)) - feat(media): require auth for relay media reads ([#1926](#1926)) ([`f308762852`](f308762)) - feat(relay): add community unarchive endpoint ([#1908](#1908)) ([`6b9641db2b`](6b9641d)) - feat(relay): gate Git web GUI separately ([#1901](#1901)) ([`34dc7dec75`](34dc7de)) - mesh: upgrade runtime, enforce membership, add shared compute provider ([#1656](#1656)) ([`54638ff4bb`](54638ff)) - Route Git scratch through configured volume ([#1884](#1884)) ([`2318b3096c`](2318b30)) - feat(relay): gate usage metrics behind stable leader ([#1814](#1814)) ([`59e9821503`](59e9821)) - Relay mesh: cross-pod tunnel + huddle transport (buzz-relay-mesh) ([#1670](#1670)) ([`ccb021d713`](ccb021d)) - feat(push): deliver accepted relay events as wakes ([#1866](#1866)) ([`bffbc5f22c`](bffbc5f)) - fix(db): resolve duplicate migration version ([#1863](#1863)) ([`08ad38a07f`](08ad38a)) - Add private product feedback sidecar ([#1857](#1857)) ([`af190c93e1`](af190c9)) - feat(relay): add durable community archival ([#1834](#1834)) ([`2b15a72675`](2b15a72)) - feat(push): add public APNs gateway ([#1770](#1770)) ([`1c006822e4`](1c00682)) - feat(relay): add atomic community ownership transfer ([#1845](#1845)) ([`52e42ccb9f`](52e42cc)) - Bound NIP-RS retention and search indexing ([#1771](#1771)) ([`1b4703021d`](1b47030)) - Add optional standalone pairing relay to Helm chart ([#1799](#1799)) ([`9b47c8548f`](9b47c85)) - fix(relay): publish membership snapshot on provisioning ([#1761](#1761)) ([`0950d392b7`](0950d39)) - feat(relay): per-community usage metrics ([#1723](#1723)) ([`620822899a`](6208228)) - refactor(desktop): remove vestigial MCP toolsets config ([#1776](#1776)) ([`dfec75b3c0`](dfec75b)) **To release:** merge this PR. The tag and build will happen automatically. Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Fixes #2637 — full analysis and reproduction there.
Problem
Audit entries are stamped and hashed with
Utc::now()(nanoseconds), then stored in aTIMESTAMPTZcolumn (microseconds).compute_hashcoverscreated_at.to_rfc3339(), and chrono emits 0/3/6/9 fractional digits depending on the value — so the digest written atservice.rs:103is computed over…T12:00:00.123456789+00:00whileverify_chainrecomputes over the…T12:00:00.123456+00:00that Postgres hands back.Every hash chain backed by a real database therefore fails verification at its first entry, on untampered data. That is not just a broken feature — it means a genuinely forged row is indistinguishable from the permanent baseline failure, so
HashMismatchcarries no signal.It is invisible in CI because all six chain tests are
#[ignore = "requires Postgres"], and the in-processhash.rstests use a fixture timestamp of2026-01-01T00:00:00Z— zero sub-seconds, the one value where the bug cannot appear.Solution
Reduce
created_atto the stored precision before hashing, so the in-memory entry and the row are byte-identical:log_inneris the only place that assignscreated_at— every caller goes throughNewAuditEntry, which carries no timestamp — so this is a single choke point. It is wrapped in alog_timestamp()helper purely so the invariant is assertable without a database.I chose truncation at the write path over the alternative (hashing a precision-independent encoding such as
timestamp_micros().to_be_bytes()). Both fix the mismatch, but truncating keeps the existing hash preimage format and gives the stronger invariant: theAuditEntryreturned fromlog()is now exactly what a later read returns.Truncation matches what actually happens on the wire — sqlx encodes
DateTime<Utc>as microseconds since the Postgres epoch, truncating — so the value hashed is the value stored.Validation
Toolchain note: built on Windows with the
x86_64-pc-windows-gnutoolchain (no MSVC linker locally).Before, against Postgres 17 with
migrations/*applied:with
HashMismatch { seq: 2 }/HashMismatch { seq: 1 }on untampered chains.After, same database:
verify_detects_tampering_within_a_communityis the one to look at: it assertsHashMismatchlands on the tampered entry'sseq. It was failing because verification already blew up on an earlier untampered row — so the assertion proving tamper detection works had never actually been exercised. It passes now.Also:
cargo test -p buzz-audit --lib(no Postgres) — 12 passed, 0 failed.cargo clippy -p buzz-audit --all-targets -- -D warnings— clean.cargo fmt -p buzz-audit -- --check— clean.New tests
Three in
hash.rs, none needing Postgres:storage_precision_drops_sub_microsecond_digits— the helper's contract, and that it is idempotent so a re-read value is unchanged.nanosecond_timestamps_cannot_survive_a_database_round_trip— asserts the digests differ. This is the trap itself, written down so the next person changing the hash preimage sees why the precision reduction is load-bearing.storage_precision_timestamps_survive_a_database_round_trip— the invariant the write path must hold.Plus
log_timestamp_carries_no_sub_microsecond_digitsinservice.rs, deliberately not#[ignore]d, so a regression on the write path is caught byjust test-unitinstead of only by Postgres-gated tests that normally never run.Compatibility
Rows written before this stay unverifiable — they always were — so there is no migration. An operator relying on an existing chain has to re-anchor.
Relationship to #2620
#2620 proposes a shared
verify_entrieswalk (anchoring, seq contiguity, tail-truncation detection) plus abuzz-admin audit verifycommand. Its Postgres-free unit tests build entries in memory and would pass regardless, but its#[ignore]Postgres tests and the operator command itself would fail on every real chain until this lands. Worth taking this first so that work has a verifiable baseline — the two changes don't overlap in code.